[test-improver] test: add edge case tests for IgnoreStringMethodReturnValueAnalyzer by Evangelink · Pull Request #9468 · microsoft/testfx

Evangelink · 2026-06-26T23:34:12Z

Goal and Rationale

IgnoreStringMethodReturnValueAnalyzer (MSTEST0055) had only 5 tests, all covering the happy path (diagnostic fires / doesn't fire based on whether the result of Contains, StartsWith, or EndsWith is used). Three distinct code paths in the analyzer's guard logic were completely untested:

Untested path	Guard
Discard assignment `_ = str.Contains(...)`	`expressionStatementOperation.Operation is not IInvocationOperation` early return
String method call inside a lambda block body	ExpressionStatement inside lambda — should still fire
String method result used as a receiver in a chained call	`IsStringMethodCall` on the outer call (not a string method) — should not fire

Approach

Added three [TestMethod] entries to IgnoreStringMethodReturnValueAnalyzerTests.cs:

Test	What it covers
`WhenDiscardAssignmentIgnoresReturnValue_NoDiagnostic`	`_ = str.Contains("Hello")` → outer operation is `IAssignmentOperation`, not `IInvocationOperation` → early return, no diagnostic
`WhenStringMethodReturnValueIgnoredInsideLambda_Diagnostic`	`() => { str.Contains("Hello"); }` → ExpressionStatement inside lambda body triggers diagnostic
`WhenStringMethodResultUsedAsReceiverForChainedCall_NoDiagnostic`	`str.Contains("Hello").GetHashCode()` → outermost ExpressionStatement operation is `GetHashCode()` on `System.Boolean`, not a string method → no diagnostic on `Contains`

Trade-offs

Purely additive — no production code changes, no new dependencies.

Test Status

All 8 tests pass (5 existing + 3 new):

Test run summary: Passed!
  total: 8
  failed: 0
  succeeded: 8
  duration: 5s 537ms

Reproducibility

.dotnet/dotnet run --project test/UnitTests/MSTest.Analyzers.UnitTests/ -f net8.0 --no-build -- \
  --filter "FullyQualifiedName~IgnoreStringMethodReturnValueAnalyzerTests"

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Test Improver workflow. · 1.1K AIC · ⌖ 25.3 AIC · ⊞ 58.2K · [◷]( · ◷)

Add this agentic workflows to your repo

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/test-improver.md@main

Add 3 edge case tests covering untested code paths in the analyzer's guard conditions: 1. WhenDiscardAssignmentIgnoresReturnValue_NoDiagnostic - `_ = str.Contains("test")` wraps an IAssignmentOperation (not IInvocationOperation), so the early-return guard fires and no diagnostic is reported. 2. WhenStringMethodReturnValueIgnoredInsideLambda_Diagnostic - Expression statements inside a lambda block body are still ExpressionStatement operations, so the analyzer fires inside the lambda just as at method level. 3. WhenStringMethodResultUsedAsReceiverForChainedCall_NoDiagnostic - `str.Contains("Hello").GetHashCode()` — the ExpressionStatement's outermost operation is GetHashCode() on System.Boolean (not a string method), so IsStringMethodCall returns false and no diagnostic is reported. The Contains result IS used as a receiver. All 8 tests pass (5 existing + 3 new). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Adds additional unit-test coverage for the MSTest analyzer IgnoreStringMethodReturnValueAnalyzer (MSTEST0055) to cover previously untested guard/shape scenarios, without changing production analyzer behavior.

Changes:

Added a test ensuring discard assignments (_ = str.Contains(...)) do not produce MSTEST0055 diagnostics.
Added a test ensuring ignored string-method return values inside a lambda block body do produce diagnostics.
Added a test ensuring chained calls where the string method result is used as a receiver (e.g. str.Contains(...).GetHashCode()) do not produce diagnostics.

Show a summary per file

File	Description
test/UnitTests/MSTest.Analyzers.UnitTests/IgnoreStringMethodReturnValueAnalyzerTests.cs	Adds three new test cases covering discard assignment, lambda-body expression statements, and chained-call receiver usage scenarios for MSTEST0055.

Review details

Files reviewed: 1/1 changed files
Comments generated: 0
Review effort level: Low

Evangelink

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Expert Review — PR #9468

Reviewed against all 22 dimensions. Changed file: test/UnitTests/MSTest.Analyzers.UnitTests/IgnoreStringMethodReturnValueAnalyzerTests.cs

Verdict table

#	Dimension	Result
1	Algorithmic Correctness	✅ LGTM
2	Threading & Concurrency	N/A
3	Security & IPC Contract Safety	N/A
4	Public API & Binary Compatibility	N/A
5	Performance & Allocations	N/A
6	Cross-TFM Compatibility	N/A
7	Resource & IDisposable Management	N/A
8	Defensive Coding at Boundaries	N/A
9	Localization & Resources	N/A
10	Test Isolation	✅ LGTM
11	Assertion Quality	✅ LGTM
12	Flakiness Patterns	✅ LGTM
13	Test Completeness & Coverage	✅ LGTM
14	Data-Driven Test Coverage	N/A
15	Code Structure & Simplification	✅ LGTM
16	Naming & Conventions	⚠️ NIT (see inline)
17	Documentation Accuracy	⚠️ NIT (see inline)
18	Analyzer & Code Fix Quality	✅ LGTM
19	IPC Wire Compatibility	N/A
20	Build Infrastructure & Dependencies	N/A
21	Scope & PR Discipline	✅ LGTM
22	PowerShell Scripting Hygiene	N/A

15/15 applicable dimensions clean. 2 NIT-level findings (no blocking or major issues).

Summary

The three new tests correctly target the three distinct guard-logic paths in IgnoreStringMethodReturnValueAnalyzer.AnalyzeExpressionStatement:

Test	Guard path exercised	Verified correct?
`WhenDiscardAssignmentIgnoresReturnValue_NoDiagnostic`	`expressionStatementOperation.Operation is not IInvocationOperation` — the outer operation is `ISimpleAssignmentOperation` (a discard), so early return fires; no diagnostic	✅
`WhenStringMethodReturnValueIgnoredInsideLambda_Diagnostic`	Block-body lambda still emits an `IExpressionStatementOperation`; analyzer fires correctly inside the lambda	✅
`WhenStringMethodResultUsedAsReceiverForChainedCall_NoDiagnostic`	Outer `IInvocationOperation` is `GetHashCode()` on `System.Boolean`, not a string method; `IsStringMethodCall` returns `false`	✅

All tests are properly isolated (no shared mutable state, no file system operations), non-flaky, and follow the project's CSharpCodeFixVerifier pattern. The using System; directive is correctly included only where System.Action is needed (the lambda test). No expression-bodied lambda gap exists because () => str.Contains(...) assigned to Action would not compile — the block-body form is the only valid pattern here.

Two NIT-level findings are attached as inline comments (naming precision on line 161, comment precision on line 170). Neither affects correctness or reviewer confidence in the PR.

…n comment Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Evangelink · 2026-06-28T19:44:39Z

🧪 Test quality grade — PR #9468

3 new test methods graded across 1 file (IgnoreStringMethodReturnValueAnalyzerTests.cs). All three tests earn top marks: they follow a clear Arrange/Act+Assert structure, use VerifyCS.VerifyAnalyzerAsync with and without diagnostic markers to assert both positive and negative analyzer behavior, and carry descriptive names that communicate the scenario and expected outcome. No improvements are needed.

Δ	Test	Grade	Band	Notes
new	`IgnoreStringMethodReturnValueAnalyzerTests. WhenExplicitDiscardAssignment_ NoDiagnostic`	A	90–100	Clear AAA; verifies the discard-assignment early-return guard produces no false positives.
new	`IgnoreStringMethodReturnValueAnalyzerTests. WhenStringMethodResultUsedAsReceiverForChainedCall_ NoDiagnostic`	A	90–100	No issues found.
new	`IgnoreStringMethodReturnValueAnalyzerTests. WhenStringMethodReturnValueIgnoredInsideLambda_ Diagnostic`	A	90–100	No issues found.

_{This advisory comment was generated automatically. Grades are heuristic

and informational — they do not block merging. Re-run with

/grade-tests.}

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Grade Tests on PR (on open / sync) workflow. · 179 AIC · ⌖ 14.2 AIC · ⊞ 45.7K · [◷]( · ◷)

Copilot AI review requested due to automatic review settings June 26, 2026 23:34

Evangelink added type/automation Created or maintained by an agentic workflow. type/test-gap Missing or insufficient tests. labels Jun 26, 2026

Copilot started reviewing on behalf of Evangelink June 26, 2026 23:34 View session

Copilot AI reviewed Jun 26, 2026

View reviewed changes

Evangelink mentioned this pull request Jun 27, 2026

[test-improver] Monthly Activity 2026-06 #9224

Open

1 task

Evangelink added the state/needs-review Awaiting review from the team. label Jun 28, 2026

Evangelink marked this pull request as ready for review June 28, 2026 04:54

This comment has been minimized.

Sign in to view

Evangelink commented Jun 28, 2026

View reviewed changes

Comment thread test/UnitTests/MSTest.Analyzers.UnitTests/IgnoreStringMethodReturnValueAnalyzerTests.cs Outdated

Comment thread test/UnitTests/MSTest.Analyzers.UnitTests/IgnoreStringMethodReturnValueAnalyzerTests.cs Outdated

0101 approved these changes Jun 28, 2026

View reviewed changes

Evangelink enabled auto-merge (squash) June 28, 2026 19:36

Address review NITs: rename discard test and correct operation type i…

1a24009

…n comment Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Evangelink merged commit eb43ddf into main Jun 28, 2026
42 of 46 checks passed

Evangelink deleted the test-assist/ignore-string-method-edge-cases-1782516482-fa3ee30ff0b2f876 branch June 28, 2026 20:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[test-improver] test: add edge case tests for IgnoreStringMethodReturnValueAnalyzer#9468

[test-improver] test: add edge case tests for IgnoreStringMethodReturnValueAnalyzer#9468
Evangelink merged 2 commits into
mainfrom
test-assist/ignore-string-method-edge-cases-1782516482-fa3ee30ff0b2f876

Evangelink commented Jun 26, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

This comment has been minimized.

Evangelink left a comment

Uh oh!

Uh oh!

Uh oh!

Evangelink commented Jun 28, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

Evangelink commented Jun 26, 2026

Goal and Rationale

Approach

Trade-offs

Test Status

Reproducibility

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Review details

Uh oh!

This comment has been minimized.

Evangelink left a comment

Choose a reason for hiding this comment

Expert Review — PR #9468

Verdict table

Summary

Uh oh!

Uh oh!

Uh oh!

Evangelink commented Jun 28, 2026

🧪 Test quality grade — PR #9468

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants